Skip to content

Overhaul constexpr <cstdlib> and <cinttypes> - #6438

Open
Stephan T. Lavavej (StephanTLavavej) wants to merge 2 commits into
microsoft:mainfrom
StephanTLavavej:what-about-second-cmath
Open

Overhaul constexpr <cstdlib> and <cinttypes>#6438
Stephan T. Lavavej (StephanTLavavej) wants to merge 2 commits into
microsoft:mainfrom
StephanTLavavej:what-about-second-cmath

Conversation

@StephanTLavavej

Copy link
Copy Markdown
Member

Followup to #6434.

  • Apply the extern "C++" inline namespace technique to __msvc_inttypes.hpp and __msvc_stdlib.hpp.
    • Reported by Cody Miller (@Codiferous). I forgot about these headers. The same technique is needed to avoid linker errors with UCRT div().
    • This also allows us to avoid the annoying "declare then #pragma function" dance.
  • Avoid unnecessary function calls in abs() and div() overloads.
    • This is a pure cleanup to improve non-optimized debug mode codegen.

Copilot AI balanced review requested due to automatic review settings September 5, 2026 19:17
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added enhancement Something can be improved high priority Important! labels Sep 5, 2026
@github-project-automation github-project-automation Bot moved this to Initial Review in STL Code Reviews Sep 5, 2026
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) moved this from Initial Review to Final Review in STL Code Reviews Sep 5, 2026
@azure-pipelines

This comment was marked as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Mixed-mode linkage regression coverage is required for both headers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 Medium severity

New issues introduced by this change (2)
Severity Finding
Medium severity stl/​inc/​__msvc_inttypes.hpp — Please cover this linkage change with a mixed-translation-unit test that compiles &lt;cinttypes&gt;
Medium severity stl/​inc/​__msvc_stdlib.hpp — Please add a mixed-translation-unit regression for this linkage change: compile one TU with…
What changed in this PR

Updates constexpr <cstdlib> and <cinttypes> functions to prevent UCRT linker conflicts and improve debug-mode code generation.

Changes:

  • Places definitions in C++ inline namespaces.
  • Implements integer abs() and div() overloads directly.
File Description
stl/​inc/​__msvc_stdlib.hpp Updates abs() and div() linkage and implementations; needs a mixed-translation-unit /Zc:cmath regression test.
stl/​inc/​__msvc_inttypes.hpp Updates imaxabs() and imaxdiv() linkage; needs a mixed-translation-unit /Zc:cmath regression test.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread stl/inc/__msvc_inttypes.hpp
Comment thread stl/inc/__msvc_stdlib.hpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved high priority Important!

Projects

Status: Final Review

Development

Successfully merging this pull request may close these issues.

2 participants